Skip to content

fix: protect global watch-task control files from non-root access#1396

Merged
myysy merged 1 commit intovolcengine:mainfrom
Hinotoi-agent:fix/watch-task-control-acl
Apr 13, 2026
Merged

fix: protect global watch-task control files from non-root access#1396
myysy merged 1 commit intovolcengine:mainfrom
Hinotoi-agent:fix/watch-task-control-acl

Conversation

@Hinotoi-agent
Copy link
Copy Markdown
Contributor

@Hinotoi-agent Hinotoi-agent commented Apr 12, 2026

OpenViking global watch-task control file ACL hardening

Title

fix: protect global watch-task control files from non-root access

Summary

This PR hardens OpenViking's watch scheduler state boundary by preventing non-root authenticated users from reading, enumerating, or overwriting the global watch-task control files stored under the shared resources scope.

  • moves internal watch scheduler state out of the broad non-root resources access model or explicitly blocks access to the watch control files
  • prevents non-root users from discovering hidden watch-task control artifacts through hidden-file listing paths
  • blocks direct content writes to watch-task control files and backup/temp variants
  • adds regression coverage for read, listing, and write paths affecting internal scheduler state

Security issues covered

issue impact severity
Non-root read access to viking://resources/.watch_tasks.json and variants cross-user disclosure of global scheduler metadata Medium
Non-root hidden-file enumeration of watch control artifacts discovery of internal scheduler control-plane files Medium
Non-root overwrite of global watch-task state cross-user integrity break against scheduler control state High

Before this PR

  • WatchManager persists scheduler state in viking://resources/.watch_tasks.json, .bak, and .tmp
  • VikingFS._is_accessible() allows non-root access to all resources-scope URIs
  • hidden files under resources are omitted from normal listing output but are still enumerable with hidden-file listing enabled
  • content writes block derived semantic artifacts like .abstract.md, .overview.md, and .relations.json, but do not block watch-task control files

After this PR

  • watch scheduler control files are treated as internal state rather than general shared resources
  • non-root callers cannot read or list the watch-task control file family
  • non-root callers cannot write the watch-task control file family through content write paths
  • tests lock in the scheduler-state trust boundary across read, listing, and write surfaces

Why this matters

The watch scheduler stores globally trusted control data in a namespace that the access-control layer currently treats as readable for any non-root authenticated user. That creates a control-plane boundary problem:

  • confidentiality: a non-root user can inspect global watch-task metadata
  • enumeration: a non-root user can discover internal scheduler files by listing hidden files
  • integrity: a non-root user can overwrite the scheduler control file if it already exists

If the scheduler later consumes attacker-modified watch state, this becomes a direct cross-user tampering primitive against global scheduler behavior.

Attack flow

non-root authenticated request
    -> filesystem/content routes over viking://resources/*
        -> broad resources-scope allow rule + no watch-task control-file denylist
            -> read / enumerate / overwrite scheduler control state
                -> disclosure and integrity impact against global watch metadata

Affected code

issue files
Global watch-task state stored in shared resources scope openviking/resource/watch_manager.py
Non-root access allowed for all resources URIs openviking/storage/viking_fs.py
Write path does not block watch-task control files openviking/storage/content_write.py

Root cause

Shared scheduler state stored in a broad-access namespace

  • WatchManager stores trusted scheduler metadata in viking://resources/.watch_tasks.json and backup/temp siblings
  • resources is a shared namespace, but these files function as internal control-plane state rather than user-facing shared content

Access control is too broad for internal resources

  • VikingFS._is_accessible() currently returns True for non-root users on all resources scope URIs
  • this scope-level allow rule does not distinguish public shared content from internal control files

Write validation protects some internals but misses watch state

  • ContentWriteCoordinator._validate_target_uri() blocks only derived semantic files
  • watch-task control files are not treated as internal scheduler metadata and remain writable if they already exist

CVSS assessment

issue CVSS v3.1 vector
Global watch-task control file read/enumeration 4.3 Medium CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:L/I:N/A:N
Global watch-task control file overwrite 8.1 High CVSS:3.1/AV:N/AC:L/PR:L/UI:N/S:U/C:L/I:H/A:L

Rationale:

  • attack complexity is low once authenticated access to the normal filesystem/content routes exists
  • privileges required are low because the issue is reachable by non-root authenticated users
  • the strongest demonstrated impact is integrity on globally trusted scheduler state
  • availability impact is kept bounded because scheduler-consumption effects may vary by deployment and validation paths

Safe reproduction steps

  1. Authenticate as a non-root user.
  2. Attempt to read:
    • viking://resources/.watch_tasks.json
    • viking://resources/.watch_tasks.json.bak
    • viking://resources/.watch_tasks.json.tmp
  3. List viking://resources with hidden-file output enabled.
  4. Attempt to write replacement JSON to viking://resources/.watch_tasks.json through the normal content-write path.
  5. Re-read the file and confirm that attacker-controlled content replaced the original scheduler metadata.

Expected vulnerable behavior

  • non-root reads succeed for the watch-task control file family
  • hidden-file listing reveals the control files under resources
  • overwrite succeeds because the file exists and is not blocked by target validation
  • derived semantic files like .abstract.md are blocked, demonstrating that this is a missing-protection gap rather than a universal no-rules write path

Validation performed

  • confirmed WatchManager persists task state at:
    • viking://resources/.watch_tasks.json
    • viking://resources/.watch_tasks.json.bak
    • viking://resources/.watch_tasks.json.tmp
  • confirmed VikingFS._is_accessible() allows non-root access to resources scope URIs
  • confirmed hidden-file listing can expose these control files when hidden output is enabled
  • confirmed write validation blocks some internal derived files but does not block the watch-task control file family
  • reproduced the read / enumerate / overwrite logic path with a source-backed harness that modeled the exact relevant access-control and write-validation logic

Changes in this PR

  • classify watch-task persistence files as internal scheduler state
  • deny non-root read and listing access to the watch-task control file family
  • deny direct content writes to the watch-task control file family, including backup and temp variants
  • add regression tests covering:
    • non-root read denial
    • hidden-file listing denial or filtering
    • non-root write denial
    • root or internal scheduler access preservation

Files changed

category files what changed
scheduler state openviking/resource/watch_manager.py retain internal storage semantics while aligning access expectations
access control openviking/storage/viking_fs.py distinguish internal watch control files from general shared resources
content write guard openviking/storage/content_write.py reject writes to watch-task control artifacts
regression tests tests/... add coverage for read, list, and write boundaries

Maintainer impact

  • patch scope is narrow and focused on internal watch scheduler state
  • user-facing shared resource behavior can remain unchanged for normal resources content
  • the fix reduces the risk of future internal-control files being exposed by broad scope-level allow rules
  • regression tests make the trust boundary easier to maintain and harder to accidentally reopen

Fix rationale

The secure default is to keep scheduler control metadata out of the general shared-resource trust model. These files are not ordinary collaborative resources; they are privileged internal state used by the watch subsystem.

A durable fix is to enforce one of these models:

  • move watch-task persistence into an internal/root-only namespace, or
  • keep the current path but explicitly treat the watch-task control file family as protected internal files across read, list, and write paths

Either approach is better than relying on hidden-file naming alone.

Type of change

  • Security fix
  • Bug fix
  • Tests
  • Documentation only
  • Refactor only

Test plan

  • Verified storage locations used by WatchManager
  • Verified non-root resources access rule in VikingFS._is_accessible()
  • Verified hidden-file listing behavior in VikingFS.ls()
  • Verified content-write validation gap in ContentWriteCoordinator._validate_target_uri()
  • Reproduced read / enumerate / overwrite behavior with a source-backed harness
  • Full containerized end-to-end reproduction on this host

Executed validation:

  • source audit of openviking/resource/watch_manager.py
  • source audit of openviking/storage/viking_fs.py
  • source audit of openviking/storage/content_write.py
  • local harness execution proving non-root access, hidden enumeration, and overwrite against the reproduced policy logic

Disclosure notes

  • claims are bounded to the verified code paths and reproduced policy behavior
  • this draft does not overclaim scheduler-side execution impact beyond the demonstrated integrity break on trusted scheduler state
  • no unrelated files need to change for the fix

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 12, 2026

CLA assistant check
All committers have signed the CLA.

@github-actions
Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🏅 Score: 95
🧪 PR contains tests
🔒 No security concerns identified
✅ No TODO sections
🔀 No multiple PR themes
⚡ No major issues detected

@github-actions
Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

@Hinotoi-agent Hinotoi-agent changed the title [security] Restrict global watch-task control files from non-root read/write access fix: protect global watch-task control files from non-root access Apr 12, 2026
@MaojiaSheng MaojiaSheng requested a review from myysy April 13, 2026 06:45
Copy link
Copy Markdown
Collaborator

@myysy myysy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks like a solid and appropriately scoped fix

@myysy myysy merged commit bdba0ef into volcengine:main Apr 13, 2026
5 of 6 checks passed
@github-project-automation github-project-automation bot moved this from Backlog to Done in OpenViking project Apr 13, 2026
@Hinotoi-agent
Copy link
Copy Markdown
Contributor Author

Thanks for the review. This PR is already merged, so I’m not patching it further here. For reference, the reviewed head SHA was 637c554a68269fb04027e6e99382ce656c7df933 and it merged as bdba0efd39ce3886f48bb5fcf61b01a41544ac57. If you still want any follow-up changes beyond the merged fix, please point me to or open a follow-up issue/PR and I can handle it there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants